Skip to content

Enable autocommit in GremlinServer#3423

Open
kenhuuu wants to merge 2 commits into
masterfrom
implicit-tx
Open

Enable autocommit in GremlinServer#3423
kenhuuu wants to merge 2 commits into
masterfrom
implicit-tx

Conversation

@kenhuuu
Copy link
Copy Markdown
Contributor

@kenhuuu kenhuuu commented May 13, 2026

This behaves the same as the TraversalOpProcessor would have in the 3.x line. All traversals are now transactional if the underlying Graph supports transactions. Traversals that aren't explicitly in a transaction are now wrapped into their own implicit transaction and the server will autocommit on sucess and rollback on failure.

Assisted-by: Kiro:claude-opus-4-6

Comment thread docs/src/dev/provider/index.asciidoc Outdated
* On error: roll back the transaction so that partial mutations are not persisted.

This auto-commit behavior ensures that users who do not use explicit transactions still get durable writes on success
and clean rollback on failure. Graph system providers implementing their own server or HTTP endpoint must replicate
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we are using "must", is there a consequence if they don't (Exception)? Can we document that?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll change the wording to should, but no there are no consequences as to the user it would just look like it worked even though it failed.

.get(GraphSONTokens.VALUEPROP).get(0)
.get(GraphSONTokens.VALUEPROP).intValue());
}
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there also a way to test stale open transactions?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would be hard to do as you can't make this happen in the normal flow since implicit transactions always attempt to rollback or commit and explicit transactions happen on a separate pool completely. We'd need some sort of mock server that has this flaw built into it so I'm going to leave this out for now if that is ok with you.

Comment on lines 432 to 441
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could any of these lines throw an error and should be in the rollback try catch in this case?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a good point. I'll move the try block up to include the script eval in case gremlin-groovy is used.

@Cole-Greer
Copy link
Copy Markdown
Contributor

VOTE +1

own transaction semantics apply. Each traversal executes within its own transaction as managed by the graph
implementation itself. Transactional requests participate in a transaction opened via `g.tx().begin()`, where the
client explicitly controls the lifecycle through `g.tx().commit()` and `g.tx().rollback()`.
Non-transactional requests (those without a `transactionId`) behave as self-contained units of work. If the underlying
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the provider docs we wrote about "graphs that don't support transactions" and here we talk about "non-transactional". I wonder if we should avoid that language and just better introduce the notion of implicit and explicit transactions and talk only in those terms. In this way, all graphs have a transaction and they all support implicit (i.e. auto-commit, sessionless, non-transactional, etc. in the old parlance) and may support explicit (i.e. manual commit, session, transactional, etc. in the old parlance). In this way we don't drag back any of the older terminology.

@xiazcy
Copy link
Copy Markdown
Contributor

xiazcy commented May 26, 2026

VOTE +1 pending comment resolution.

kenhuuu added 2 commits May 27, 2026 14:32
This behaves the same as the TraversalOpProcessor would have
in the 3.x line. All traversals are now transactional if the
underlying Graph supports transactions. Traversals that aren't
explicitly in a transaction are now wrapped into their own
implicit transaction and the server will autocommit on sucess
and rollback on failure.

Assisted-by: Kiro:claude-opus-4-6
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants